fixed infered type from useServerFn + added type test #5367
Conversation
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 6m 6s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 9s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-05 11:06:00 UTC
WalkthroughRefactors useServerFn in React and Solid to a conditional, arity-aware return type and named handler; adds example wiring of a server-side mutation in the React example and new d.ts tests for React and Solid demonstrating useServerFn with React Query and various invocation shapes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Home Component
participant RQ as React Query
participant Adapter as useServerFn(handler)
participant Server as Server Function (greetUser)
User->>UI: Click "Say hi"
UI->>RQ: mutate({ name: "TanStack" })
RQ->>Adapter: call mutationFn(args)
Adapter->>Server: POST /greetUser { name }
Server-->>Adapter: { message }
Adapter-->>RQ: { message }
RQ-->>UI: onSuccess(data)
UI-->>User: Render greeting
sequenceDiagram
autonumber
participant Cmp as Component
participant Hook as useServerFn(handler)
participant Router as Router
participant Server as Server Function
Cmp->>Hook: call handler(...args)
Hook->>Server: invoke
alt Redirect thrown
Server-->>Hook: RedirectError
Hook->>Router: navigate(redirectTarget)
Router-->>Hook: navigation result
Hook-->>Cmp: navigation result (cast as AwaitedReturn)
else Success
Server-->>Hook: result
Hook-->>Cmp: result (AwaitedReturn)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/react/start-basic-react-query/src/routes/index.tsx(1 hunks)packages/react-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)packages/react-start/src/useServerFn.ts(1 hunks)packages/solid-start/src/useServerFn.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-start/src/useServerFn.tspackages/solid-start/src/useServerFn.tspackages/react-start/src/tests/useServerFnMutation.test-d.tsxexamples/react/start-basic-react-query/src/routes/index.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/react-start/src/useServerFn.tspackages/solid-start/src/useServerFn.tspackages/react-start/src/tests/useServerFnMutation.test-d.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
examples/react/start-basic-react-query/src/routes/index.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/react/start-basic-react-query/src/routes/index.tsx
🧬 Code graph analysis (4)
packages/react-start/src/useServerFn.ts (1)
packages/solid-start/src/useServerFn.ts (1)
useServerFn(14-41)
packages/solid-start/src/useServerFn.ts (1)
packages/react-start/src/useServerFn.ts (1)
useServerFn(15-45)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (1)
packages/react-start/src/useServerFn.ts (1)
useServerFn(15-45)
examples/react/start-basic-react-query/src/routes/index.tsx (2)
e2e/react-start/server-functions/src/routes/formdata-redirect/index.tsx (1)
greetUser(16-33)packages/react-start/src/useServerFn.ts (1)
useServerFn(15-45)
🪛 Biome (2.1.2)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx
[error] 11-11: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 9-10: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
|
It looks there are few tweaks I need to do |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)packages/react-start/src/useServerFn.ts(1 hunks)packages/solid-start/src/useServerFn.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-start/src/useServerFn.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start/src/useServerFn.tspackages/react-start/src/tests/useServerFnMutation.test-d.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/solid-start/src/useServerFn.tspackages/react-start/src/tests/useServerFnMutation.test-d.tsx
🧬 Code graph analysis (2)
packages/solid-start/src/useServerFn.ts (1)
packages/react-start/src/useServerFn.ts (1)
useServerFn(21-51)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (1)
packages/react-start/src/useServerFn.ts (1)
useServerFn(21-51)
🪛 Biome (2.1.2)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx
[error] 11-11: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 9-10: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 20-21: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 24-25: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (3)
4-10: Consider removing redundant Promise wrapping.The
await Promise.resolve()is unnecessary since the handler is alreadyasync. Simplifying toreturn { message: \Hello ${data.name}!` }` would be cleaner.Apply this diff:
const serverFn = createServerFn({ method: 'POST' }) .inputValidator((input: { name: string }) => input) .handler(async ({ data }) => { - return await Promise.resolve({ - message: `Hello ${data.name}!`, - }) + return { + message: `Hello ${data.name}!`, + } })
12-16: Consider removing redundant Promise wrapping.Same as above - the
await Promise.resolve()is unnecessary. Simplify toreturn { ok: true as const }.Apply this diff:
const optionalServerFn = createServerFn().handler(async () => { - return await Promise.resolve({ - ok: true as const, - }) + return { + ok: true as const, + } })
18-29: Consider adding explicit type assertions for robustness.While TypeScript compilation verifies the types, using explicit type assertions (e.g.,
expectTypeOffrom vitest or tsd) would make the test more robust and self-documenting. This would catch regressions even if TypeScript's inference changes subtly.Example approach:
import { expectTypeOf } from 'vitest' export function UseServerFnMutationRegressionComponent() { const mutation = useMutation({ mutationFn: useServerFn(serverFn), onSuccess: (data, variables) => { expectTypeOf(data).toHaveProperty('message') expectTypeOf(data.message).toBeString() expectTypeOf(variables?.data?.name).toEqualTypeOf<string | undefined>() }, }) void mutation return null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)packages/solid-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start/src/tests/useServerFnMutation.test-d.tsxpackages/react-start/src/tests/useServerFnMutation.test-d.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/solid-start/src/tests/useServerFnMutation.test-d.tsxpackages/react-start/src/tests/useServerFnMutation.test-d.tsx
🧬 Code graph analysis (2)
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (1)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (1)
useOptionalServerFnRegressionHook(31-40)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (2)
packages/react-start/src/useServerFn.ts (1)
useServerFn(21-51)packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (1)
useOptionalServerFnRegressionHook(28-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (6)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (1)
31-40: LGTM!This hook correctly tests the optional parameter scenario, verifying that
useServerFnreturns a callable handler that properly types the result. The test covers both invocation styles (handler()andhandler(undefined)) and verifies the result shape.packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (5)
1-2: LGTM!The imports are correct for testing server function type inference.
3-9: LGTM!The server function definition with input validation and handler is correctly structured. The handler properly destructures the validated input from the
dataproperty.
11-15: LGTM!The optional server function definition is correct. The use of
as constfor theokproperty ensures precise type inference (truerather thanboolean).
24-24: Clarify the intent of voiding the handler reference.This line voids the handler reference itself rather than calling it. In the React version (relevant code snippet from
packages/react-start/src/tests/useServerFnMutation.test-d.tsx), the equivalent test calls the handler:void optionalHandler()andvoid optionalHandler(undefined).If the intent is to test that the handler can be used in void expression contexts, consider making this explicit with a comment. Otherwise, consider calling the handler like
void handler({ name: 'TanStack' })to match the pattern used for the optional handler (lines 35-36).
28-37: LGTM!The optional server function usage correctly tests various call signatures (no arguments, explicit
undefined) and verifies that the return type is properly inferred (accessingresult.ok). This aligns well with the React equivalent in the relevant code snippets.
| export function UseServerFnRegressionComponent() { | ||
| const handler = useServerFn(serverFn) | ||
|
|
||
| handler({ data: { name: 'TanStack' } }).then((result) => { |
There was a problem hiding this comment.
Fix the input type mismatch.
The handler is being called with { data: { name: 'TanStack' } }, but the input validator (line 4) expects { name: string }. The data property wrapper is used server-side within the handler, not in the client-side call signature.
Apply this diff to fix the call:
- handler({ data: { name: 'TanStack' } }).then((result) => {
+ handler({ name: 'TanStack' }).then((result) => {
result.message
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handler({ data: { name: 'TanStack' } }).then((result) => { | |
| handler({ name: 'TanStack' }).then((result) => { | |
| result.message | |
| }) |
🤖 Prompt for AI Agents
In packages/solid-start/src/tests/useServerFnMutation.test-d.tsx around line 20,
the test calls handler({ data: { name: 'TanStack' } }) but the input validator
expects a plain { name: string }; remove the unnecessary data wrapper and call
handler({ name: 'TanStack' }) instead so the argument matches the declared input
type used by the validator.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (2)
4-10: Consider simplifying the async handler.The
await Promise.resolve()wrapper is redundant since async functions automatically wrap return values in a Promise. For clarity, consider simplifying to:- .handler(async ({ data }) => { - return await Promise.resolve({ - message: `Hello ${data.name}!`, - }) - }) + .handler(async ({ data }) => { + return { + message: `Hello ${data.name}!`, + } + })
12-16: Consider simplifying the async handler.Similar to the previous handler, the
await Promise.resolve()wrapper is unnecessary:-const optionalServerFn = createServerFn().handler(async () => { - return await Promise.resolve({ - ok: true as const, - }) -}) +const optionalServerFn = createServerFn().handler(async () => { + return { + ok: true as const, + } +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-start/src/tests/useServerFnMutation.test-d.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/react-start/src/tests/useServerFnMutation.test-d.tsx
🧬 Code graph analysis (1)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (2)
packages/react-start/src/useServerFn.ts (1)
useServerFn(21-51)packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (1)
useOptionalServerFnRegressionHook(28-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (3)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (3)
1-2: LGTM!Imports are appropriate for testing the React Query integration with server functions.
18-29: Excellent regression test for issue #5352.This component effectively validates that
useServerFninlined withinuseMutationpreserves correct type inference. TheonSuccesscallback accessingdata.messageandvariables.data.namewould fail to compile if types regressed tounknown, making this a robust type-level guard against future regressions.
31-40: LGTM!This hook properly tests the optional parameter handling for
useServerFn, ensuring that both zero-argument and explicitundefinedinvocations work correctly while preserving type inference for the result.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (1)
16-18: Fix the input type mismatch.The handler is being called with
{ data: { name: 'TanStack' } }, but the input validator (line 4) expects{ name: string }. Thedataproperty wrapper is used server-side within the handler, not in the client-side call signature.Apply this diff to fix the call:
- handler({ data: { name: 'TanStack' } }).then((result) => { + handler({ name: 'TanStack' }).then((result) => { result.message })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)packages/solid-start/src/tests/useServerFnMutation.test-d.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-start/src/tests/useServerFnMutation.test-d.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx
🧬 Code graph analysis (1)
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (1)
packages/react-start/src/tests/useServerFnMutation.test-d.tsx (1)
useOptionalServerFnRegressionHook(27-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (4)
packages/solid-start/src/tests/useServerFnMutation.test-d.tsx (4)
1-2: LGTM!The imports are correctly structured for a test file in this location.
3-7: LGTM!The server function definition correctly uses input validation and the handler pattern.
9-11: LGTM!The optional server function definition is appropriate for testing scenarios without input validation.
24-33: LGTM!The optional server function test correctly exercises various invocation patterns, including no arguments, undefined, and void contexts. This provides good coverage for the type inference scenarios.
| result.message | ||
| }) | ||
|
|
||
| void handler |
There was a problem hiding this comment.
Incomplete void context test.
Line 20 uses void handler which only references the handler without calling it. This doesn't effectively test the handler's behavior in a void context. Based on the React equivalent test (lines 31-32 in the relevant snippets), this should call the handler.
Apply this diff to call the handler in a void context:
- void handler
+ void handler({ name: 'TanStack' })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void handler | |
| void handler({ name: 'TanStack' }) |
🤖 Prompt for AI Agents
In packages/solid-start/src/tests/useServerFnMutation.test-d.tsx around line 20,
the test currently uses "void handler" which only references the handler instead
of invoking it; change this to call the handler in a void context (e.g., use
"void handler()" so the handler is executed) to match the React equivalent test
and properly verify void-context behavior.
|
I think I finished on this, but one test is failing, it doesn't look related to my changes, could someone have a look? |
Summary
useServerFntyping in React/Solid so inlineuseMutation({ mutationFn: useServerFn(fn) })resolves to the awaited payload instead of
unknown.test-d.tsxregression exercising the React Query reproductionSummary by CodeRabbit
New Features
Refactor
Tests